*: Replace Generator.Spec() with Generator.Config #1008
Conversation
libpod/container_internal.go
Outdated
| @@ -1552,5 +1552,5 @@ func (c *Container) setupOCIHooks(ctx context.Context, g *generate.Generator) (e | |||
| return nil, nil | |||
| } | |||
|
|
|||
| return manager.Hooks(g.Spec(), c.Spec().Annotations, len(c.config.UserVolumes) > 0) | |||
| return manager.Hooks(config, config.Annotations, len(c.config.UserVolumes) > 0) | |||
There was a problem hiding this comment.
This is a semantic change from before - we were using the annotations from the config supplied when creating the container libpod, not the annotations in the final/completed spec used to initialize the OCI runtime. I don't necessarily think this is a bad change - it's counterintuitive for people to see an environment variable in the container at runtime but not be able to trigger hooks off it - but work noting.
There was a problem hiding this comment.
This is a semantic change from before...
Oops, I only meant to adjust the first argument. Fixed with 4f49ffd5 -> 4e7d15f.
I don't necessarily think this is a bad change...
I don't mind if you want to file a follow-up PR to make this semantic pivot; but I don't think it should be mixed up with this PR's refactor.
Catching up with opencontainers/runtime-tools@84a62c6a (generate: Move Generator.spec to Generator.Config, 2016-11-06, containers#266, v0.6.0), now that we've bumped runtime-tools in f6c0fc1 (Vendor in latest runtime-tools, 2018-06-26, containers#1007). Signed-off-by: W. Trevor King <[email protected]>
wking
force-pushed
the
runtime-tools-spec-deprecated
branch
from
4f49ffd to
4e7d15f
Compare
|
LGTM |
|
LGTM |
|
📌 Commit 4e7d15f has been approved by |
|
⚡ Test exempted: pull fully rebased and already tested. |
github-actions
bot
added
the
locked - please file new issue/PR
4 participants
Catching up with opencontainers/runtime-tools@84a62c6a (opencontainers/runtime-tools#266), now that we've bumped runtime-tools in f6c0fc1 (#1007).
CC @baude.